Skip to content

feat: Enable trust remote code in CI. - #498

Open
dushyantbehl wants to merge 3 commits into
torch-spyre:mainfrom
dushyantbehl:fix-trust-remote-code
Open

feat: Enable trust remote code in CI.#498
dushyantbehl wants to merge 3 commits into
torch-spyre:mainfrom
dushyantbehl:fix-trust-remote-code

Conversation

@dushyantbehl

@dushyantbehl dushyantbehl commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Fixes #482

Builds on top of #478 and requires #478 to be merged before this PR.

@dushyantbehl

dushyantbehl commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

FYI @BenjSz @arielge

BenjSz
BenjSz reviewed Sep 9, 2026
View reviewed changes
Comment thread tests/model_registry.py Outdated
@BenjSz

BenjSz commented Sep 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Don't forget to remove the model from the excluded.yaml list when we merge.

@BenjSz

BenjSz commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

st_backend file should also contain the handing of trust_remote_code

@BenjSz

BenjSz commented Sep 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

tests/spyre/test_e2e_question_answering_compare_spyre.py
tests/spyre/test_e2e_masked_lm_compare_spyre.py
tests/spyre/test_e2e_token_classification_compare_spyre.py
tests/cpu/test_question_answering_cpu_accuracy.py
tests/cpu/test_masked_lm_cpu_accuracy.py
tests/cpu/test_token_classification_cpu_accuracy.py
tests/cpu/_seq_classification_helpers.py
tests/spyre/test_load_spyre.py
tests/_vision_helpers.py
tests/spyre/test_vlm_e2e_spyre.py
tests/spyre/test_e2e_smoke_vision_spyre.py
tests/cpu/test_vlm_e2e_cpu.py
tests/spyre/_seq_classification_helpers.py
tests/spyre/test_multicard_spyre.py
test_dspark_draft_spyre.py

You do not handle the trust_remote_code in these scripts when you call from_pretrained()

@dushyantbehl

dushyantbehl commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @BenjSz let me update code and check the other files.

BenjSz
BenjSz requested changes Sep 9, 2026
View reviewed changes
Comment thread tests/cpu/test_adapter_cpu_accuracy.py Outdated
def test_auto_loader(model_path):
auto_spyre_model = sys.modules["hf_adapters.auto_spyre_model"]
tokenizer = AutoTokenizer.from_pretrained(model_path)
trust_remote_code = model_path in REMOTE_CODE_PATHS

@BenjSz BenjSz Sep 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A general comment that applies to all the files.

The trust_remote_code should be a parameter of the methods.

Like that, if someone is running the test with a model_path parameter (without passing via model_registry or via the CI) , it still can passes the parameter trust_remote_code.

Keeping the value of trust_remote_code as dependant of REMOTE_CODE_PATHS makes it dependent of the models defined in model_registry.py and we do not want that.

@dushyantbehl dushyantbehl Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A general comment that applies to all the files.

The trust_remote_code should be a parameter of the methods.

Like that, if someone is running the test with a model_path parameter (without passing via model_registry or via the CI) , it still can passes the parameter trust_remote_code.

Keeping the value of trust_remote_code as dependant of REMOTE_CODE_PATHS makes it dependent of the models defined in model_registry.py and we do not want that.

@BenjSz I have update the code to have trust_remote_code as a parameter of the test methods and have also added --trust_remote_code over conftest commandline in case users need to use a single such model test.
I am hoping once the clip PR #502 is merged the CI should be green.

Comment thread tests/model_registry.py Outdated
BenjSz
BenjSz requested changes Sep 9, 2026
View reviewed changes
Comment thread tests/cpu/test_embed_cpu_accuracy.py Outdated
@dushyantbehl

dushyantbehl commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

@BenjSz please see the updated code and my comment on clarifying the design choice for maintaining a separate REMOTE_CODE_PATHS and let me know your thoughts.

@dushyantbehl
enable trust remote code models in test framework
c9259e4 
Signed-off-by: Dushyant Behl <myselfdushyantbehl@gmail.com>
@dushyantbehl
update code and add trust_remote_code to remaining files
bad4be7 
Signed-off-by: Dushyant Behl <myselfdushyantbehl@gmail.com>
BenjSz
BenjSz previously approved these changes Sep 10, 2026
View reviewed changes

@BenjSz BenjSz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dushyantbehl
Trust remote code should be a method parameter
bb11adb 
Signed-off-by: Dushyant Behl <myselfdushyantbehl@gmail.com>
@anubhavjana anubhavjana mentioned this pull request Sep 11, 2026
Closed
@anubhavjana

anubhavjana commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

@BenjSz @arielge Fix to the failing CI is in #511 but it depends on #502 to fix CLIP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BenjSz BenjSz BenjSz left review comments
@arielge arielge Awaiting requested review from arielge arielge is a code owner
@kiszk kiszk Awaiting requested review from kiszk kiszk is a code owner
@Mohini-Shrivastava Mohini-Shrivastava Awaiting requested review from Mohini-Shrivastava Mohini-Shrivastava is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add trust_remote_code support in model_registry.py and wire it through CI tests

3 participants

@dushyantbehl @BenjSz @anubhavjana